Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(data-migrate): fix stdout capture and failed step functions #749

Merged
merged 1 commit into from
Dec 3, 2024
Merged

fix(data-migrate): fix stdout capture and failed step functions #749

merged 1 commit into from
Dec 3, 2024

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Dec 3, 2024 •
edited
Loading

Related to #726

Changes

  • Fixes stdout capture so that the task fails if the last sync contains output.
  • Signals to the step function to fail the step on exceptions to avoid the step function hanging until timeout.

@mmalenic mmalenic self-assigned this Dec 3, 2024
@mmalenic mmalenic added the fix label Dec 3, 2024
@victorskl victorskl linked an issue Dec 3, 2024 that may be closed by this pull request
Add data mover service #721
Closed
2 tasks
@mmalenic mmalenic merged commit 5e3da5e into main Dec 3, 2024
6 checks passed
@mmalenic mmalenic deleted the fix/data-mover branch December 3, 2024 06:58
@alexiswl
Copy link
Member

alexiswl commented Dec 3, 2024

Sorry, just saw this, but you might be better off using the 'returncode' attribute of subprocess.run to determine if aws s3 sync has succeeded.

See

orcabus/lib/workload/stateless/stacks/stacky-mcstackface/glue-constructs/clag/part_2/fastq-list-rows-event-shower/lambdas/get_fastqc_stats/run_fastqc.py

Lines 129 to 139 in ed949c5

run_fastqc_proc = subprocess.run(
["bash", temp_file_obj.name],
cwd=working_dir.name,
capture_output=True
)
if not run_fastqc_proc.returncode == 0:
logger.error(f"Run FastQC Proc failed with return code {run_fastqc_proc.returncode}")
logger.error(f"Run FastQC Proc failed with stderr {run_fastqc_proc.stderr.decode()}")
logger.error(f"Run FastQC Proc failed with stdout {run_fastqc_proc.stdout.decode()}")
raise ChildProcessError
as an example

Also you can use .decode() to convert the stderr and stdout to strings from 'byte' types, warning though, the stderr and stdout attributes of the process are 'generators', so if you try and collect it a second time it will be empty.

@mmalenic
Copy link
Member Author

mmalenic commented Dec 3, 2024

you might be better off using the 'returncode' attribute of subprocess.run to determine if aws s3 sync has succeeded.

check=True should also be checking if the command succeeded right? I think I have to compare the stdout anyway because I need to check if aws s3 sync contains empty output vs non-empty output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reisingerf reisingerf reisingerf approved these changes

@victorskl victorskl victorskl approved these changes

Assignees

@mmalenic mmalenic

Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add data mover service
4 participants
@mmalenic @alexiswl @victorskl @reisingerf